Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proxy_* params were removed from class apt #443

Merged
merged 1 commit into from
Feb 26, 2015

Conversation

underscorgan
Copy link

Add them to PPA since they were being used there, and add a placeholder
example for setting up the proxy files.

@underscorgan underscorgan added this to the 2.0 milestone Feb 25, 2015
@@ -5,6 +5,8 @@
$options = $::apt::ppa_options,
$package_name = $::apt::ppa_package,
$package_manage = false,
$proxy_host = undef,
$proxy_port = 8080,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, I've started disliking having multiple variables that are an obvious pair/entity as multiple switches on the class and instead have taken to doing something like this:

class apt::params {
  $proxy = { 'host': undef, 'port': 8080 }
}

define apt::ppa (
  $proxy = {}
)  {
  $_proxy = merge($apt::proxy, $proxy)
}

This allows the user to still only specify proxy => { 'host': '127.0.0.1' } and get the port for free.

What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daenney I don't hate that, but I am a little confused with your hash syntax. I've only remembering seeing puppet hashes with => as the separator between key/value (and also this). Is this something new?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I've just been doing too much of non-Ruby/Puppet as of late. I meant hash rockets.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, cool, just wanted to make sure I didn't miss something new :) I'll get this updated.

@daenney
Copy link

daenney commented Feb 25, 2015

Mmm, we actually have no tests covering this change.

@daenney daenney assigned underscorgan and unassigned daenney Feb 25, 2015
Add them to PPA since they were being used there, and add a placeholder
example for setting up the proxy files.
@underscorgan underscorgan assigned daenney and unassigned underscorgan Feb 25, 2015
daenney added a commit that referenced this pull request Feb 26, 2015
proxy_* params were removed from class apt
@daenney daenney merged commit e588ab6 into puppetlabs:next Feb 26, 2015
@underscorgan underscorgan deleted the proxy_updates branch May 8, 2015 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants